Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@infrastructure/compute.py`:
- Around line 476-480: The server and worker task definitions both use the
shared variable model_trainer_server_worker_image_uri, causing image race
conditions; change the deployment to use distinct image identifiers (e.g.,
model_trainer_server_image_uri and model_trainer_worker_image_uri) or pin
immutable digests for each stage and replace
model_trainer_server_worker_image_uri in container_definitions and the other
occurrence (the similar container_definitions block around the second mention)
with the appropriate per-role image variable so server and worker always get the
correct image.

In `@maskfile.md`:
- Around line 213-214: The ECS waiter timeout is too short (Delay: 10,
MaxAttempts: 6 → 60s); update the aws ecs wait services-stable command to allow
up to ~5 minutes by increasing the waiter-config (e.g., keep "Delay": 10 and set
"MaxAttempts": 30 or set "Delay": 20 and "MaxAttempts": 15) so the ecs wait
services-stable --cluster "$cluster" --services "$service" uses the longer
timeout for stabilization.

In `@models/tide/src/tide/workflow.py`:
- Around line 41-45: The train_model signature currently types configuration as
plain dict | None; tighten it by declaring and using a more specific
configuration type (e.g., a TypedDict named ModelConfiguration or at minimum
dict[str, Any]) and update the parameter type to ModelConfiguration | None (or
dict[str, Any] | None) to match the shape seen in DEFAULT_CONFIGURATION; update
any imports (typing.TypedDict/Any) and adjust usages in train_model to use the
new type alias so type checkers can validate keys referenced inside the
function.

In `@models/tide/tests/test_deploy.py`:
- Around line 25-27: The test is missing an assertion for the new timezone
parameter: update the assertions in the test (where call_kwargs,
call_kwargs["parameters"], cron and LOOKBACK_DAYS are checked) to include assert
call_kwargs["parameters"]["timezone"] == "America/New_York" so the deployed
job's timezone is validated alongside cron, base_url, and lookback days.

In `@models/tide/tests/test_workflow.py`:
- Around line 171-179: The two tests test_train_model_returns_model_and_data and
test_train_model_uses_default_configuration call train_model() without
overriding epoch_count and thus depend on production defaults; update these
tests to either inject a cheap test configuration (e.g., pass epoch_count=1 or a
TestConfig object) into train_model or stub/mock the model.fit path so training
is cheap and deterministic—look for train_model, epoch_count and any internal
fit/training call to patch or parameterize, and make the same change for the
other affected tests (the ones around the 201-207 range) so all unit tests pin a
minimal training budget.

---

Outside diff comments:
In `@infrastructure/compute.py`:
- Around line 363-380: You renamed the Pulumi logical resource and the RDS
identifier (new resource model_trainer_state with identifier
"fund-model-trainer-state"), which causes Pulumi to create a new RDS instance
and orphan the existing "fund-prefect" DB; to fix, either revert the Pulumi
logical name and identifier back to the original values (restore the resource
name prefect_database and identifier "fund-prefect") so Pulumi will perform an
in-place update, or implement an explicit snapshot/restore migration: take a
snapshot of the existing instance, restore that snapshot into the desired new
instance identifier, import/replace the Pulumi resource state as needed, and
ensure deletion_protection/backups are configured so the original instance isn’t
inadvertently destroyed; update the code around model_trainer_state, identifier,
and any references to the original prefect_database/fund-prefect to match the
chosen approach.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@models/tide/src/tide/workflow.py`:
- Around line 26-38: DEFAULT_CONFIGURATION is exported as a regular mutable dict
which can be accidentally mutated; replace it with an immutable mapping (e.g.,
wrap the literal with types.MappingProxyType or expose a
get_default_configuration() that returns a shallow copy) so callers cannot
change module-level defaults. Update the top-level symbol DEFAULT_CONFIGURATION
to be the immutable MappingProxyType of the existing dict (or replace usages to
call get_default_configuration()), import types.MappingProxyType if needed, and
ensure any tests or code that previously mutated DEFAULT_CONFIGURATION instead
obtain a mutable copy via dict(DEFAULT_CONFIGURATION) or the getter.

---

Duplicate comments:
In `@maskfile.md`:
- Around line 220-221: The ECS waiter timeout is too short for service
stabilization; update the aws ecs wait services-stable invocation (the command
using "$cluster" and "$service") to increase the waiter-config Delay/MaxAttempts
values so the total wait is much longer (e.g., Delay 15 and MaxAttempts 40 for
~10 minutes) to avoid flaky deploy failures during rolling updates; modify the
waiter-config JSON accordingly to reflect the new Delay and MaxAttempts.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.claude/commands/update-pull-request.md:
- Around line 307-330: The script currently uses hardcoded placeholder thread
IDs in the threads list and calls urllib.request.urlopen without a timeout or
error checking; update it to read real thread IDs from the scratchpad file
instead of the hardcoded "threads" list, add a timeout to the network call (use
the timeout parameter on urllib.request.urlopen), and validate the GraphQL
response by checking for an "errors" key and HTTP error conditions before using
data; also wrap the token retrieval, network call (urllib.request.urlopen) and
JSON parsing in try/except to log/print failures and continue processing other
thread IDs so one error doesn’t hang or abort the whole script.
- Around line 56-60: The REST gh api calls that save PR data (the commands
writing to "${SCRATCHPAD}/pr_meta_raw.json",
"${SCRATCHPAD}/pr_comments_raw.json", "${SCRATCHPAD}/pr_reviews_raw.json", and
"${SCRATCHPAD}/pr_review_comments_raw.json") only fetch the first page; add the
--paginate flag to each of those gh api invocations so all pages are retrieved.
For the GraphQL query that extracts review threads (the block capped at 100),
implement cursor-based pagination (use endCursor/hasNextPage and loop until
hasNextPage is false) so all threads are fetched, and when using jq to write
combined outputs, add --slurp to produce a single JSON array to avoid fragmented
output. Ensure the same --paginate and pagination approach is applied to the
other REST/GraphQL review/comments fetches referenced in the file (including the
review-threads section).

---

Outside diff comments:
In @.claude/commands/update-pull-request.md:
- Around line 32-74: The script currently runs gh and jq invocations (e.g., gh
api calls that write pr_meta_raw.json, pr_comments_raw.json,
pr_reviews_raw.json, pr_review_comments_raw.json, check_runs_raw.json and the
INLINE_QUERY GraphQL call that writes thread_ids_raw.json) without strict
failure handling; add set -euo pipefail at the top of each bash block that
performs external commands (the scratchpad creation block that sets SCRATCHPAD
and the blocks that call gh/jq and that perform jq transformations) so the
script exits on errors/undefined variables and failed pipes, and also ensure any
jq transformations that write derived files validate input (e.g., check files
exist and non-empty after jq runs) before proceeding; update blocks around
SCRATCHPAD creation, the gh api fetches, and the jq processing/INLINE_QUERY
usage accordingly.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/launch_infrastructure.yaml:
- Around line 74-83: The pre-stack ECS service deploy step (the step named
"Deploy ${{ matrix.application }}-${{ matrix.stage }}" that runs "mask
infrastructure image deploy") can fail if services or infra are renamed; reorder
the workflow so the Pulumi stack operation ("mask infrastructure stack up" run
in the launch_infrastructure job) happens before any per-service deploys, i.e.
make the "launch_infrastructure" job run immediately after build/push and have
the matrix/service deploy steps depend on its completion (or move "mask
infrastructure image deploy" into a job that lists launch_infrastructure as a
prerequisite), ensuring "mask infrastructure image deploy" only runs after "mask
infrastructure stack up".
- Around line 4-7: The workflow currently triggers on every push to master;
narrow the trigger by adding on.push.paths to only infra/service-relevant
patterns (e.g. infrastructure/**, pulumi/**, deploy/**, charts/**,
scripts/pulumi/**) and/or on.push.paths-ignore for docs/metadata (e.g. **/*.md)
so the Pulumi job only runs for relevant changes; update the YAML's on.push
block (keeping branches: [master]) to include these path patterns to reduce
unnecessary full infra runs.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/launch_infrastructure.yaml:
- Around line 98-154: The deploy_images job duplicates the matrix and path
filter definitions; consolidate by extracting the matrix and path-filter config
into YAML anchors (e.g., define a top-level anchor like &matrix-config for the
matrix entries and &service-paths for the paths list) and then reference them in
deploy_images (strategy.matrix: <<: *matrix-config) and in the Check for service
changes step (use the anchored list for the filters input), ensuring job name
deploy_images and the paths-filter step id changes still consume the anchored
values; alternatively create a reusable workflow that accepts the matrix as an
input and have both deploy_images and build_and_push_images call it.

# RDS PostgreSQL for Prefect database
model_trainer_state = aws.rds.Instance(
    "model_trainer_state",
    identifier="fund-model-trainer-state",
    engine="postgres",
    engine_version="14",
    instance_class="db.t3.micro",
    allocated_storage=20,
    db_name="prefect",
    username="prefect",
    manage_master_user_password=True,
    db_subnet_group_name=prefect_rds_subnet_group.name,
    vpc_security_group_ids=[prefect_rds_security_group.id],
    skip_final_snapshot=False,
    final_snapshot_identifier=f"fund-model-trainer-state-final-{pulumi.get_stack()}",
    backup_retention_period=7,
    storage_encrypted=True,
    deletion_protection=True,
    tags=tags,
)

# ElastiCache Redis for Prefect messaging
model_trainer_broker = aws.elasticache.Cluster(
    "model_trainer_broker",
    cluster_id="fund-model-trainer-broker",
    engine="redis",
    engine_version="7.0",
    node_type="cache.t3.micro",
    num_cache_nodes=1,
    subnet_group_name=prefect_elasticache_subnet_group.name,
    security_group_ids=[prefect_redis_security_group.id],
    tags=tags,
)